Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

[WIP] Fix precision issue of test case test_rnnrelu_bidirectional and test_rnnrelu_sym - #11445

Closed
lihaofd wants to merge 2 commits into
apache:masterfrom
lihaofd:relu-fix
Closed

[WIP] Fix precision issue of test case test_rnnrelu_bidirectional and test_rnnrelu_sym#11445
lihaofd wants to merge 2 commits into
apache:masterfrom
lihaofd:relu-fix

Conversation

@lihaofd

@lihaofd lihaofd commented Jun 28, 2018 •
edited
Loading

Copy link
Copy Markdown

Description

In this PR, it fixed precision issue of test case #11410 test_rnnrelu_bidirectional and test_rnnrelu_sym. @pengzhao-intel, @TaoLv

Feature changes

New features

  • Fixed issue of precision issue of test case Flaky test: test_operator_gpu.test_rnnrelu_bidirectional #11410 test_rnnrelu_bidirectional and test_rnnrelu_sym. Sometimes, the data comparison will fail because of tolerance of precision, like 0.043388 and 0.04405. In our testing environment, the biggest testing result deviation is around difference between 0.027... and 0.037... sometimes. Adjust the tolerance to fix the issue.

Checklist

  • Passed code style checking (make lint).
  • All changes have test coverage.
  • Code is well-documented.

@lihaofd lihaofd changed the title Fix precision issue of test case [WIP] Fix precision issue of test case test_rnnrelu_bidirectional Jun 28, 2018
@lihaofd lihaofd changed the title [WIP] Fix precision issue of test case test_rnnrelu_bidirectional [WIP] Fix precision issue of test case test_rnnrelu_bidirectional and test_rnnrelu Jun 28, 2018
@lihaofd lihaofd changed the title [WIP] Fix precision issue of test case test_rnnrelu_bidirectional and test_rnnrelu [WIP] Fix precision issue of test case test_rnnrelu_bidirectional and test_rnnrelu_sym Jun 28, 2018
@lihaofd lihaofd closed this Jun 28, 2018
@vandanavk

vandanavk commented Aug 2, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

@lihaofd any reason for closing this PR?

@lihaofd

lihaofd commented Aug 3, 2018

Copy link
Copy Markdown
Author

@vandanavk. We are still looking into the issue and check if there is other way to fix it. @TaoLv @pengzhao-intel

@vandanavk

vandanavk commented Aug 7, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

@lihaofd
Any pointers so far?

The issue is always reproducible with seed 272273578, FusedRNN with 2 layers and bidirectional=True. I tried out a few experiments - the issue doesn't occur with 3 layers in test_rnnrelu_bidirectional or when the FusedRNN is unfused.

Also, any reason for
T, N, I, H = 5, 20, 200, 200 in the case of test_rnnrelu_bidirectional and test_rnnrelu_sym instead of T, N, I, H = 5, 20, 800, 800 for test_rnnrelu_bidirectional

@vandanavk

vandanavk commented Aug 8, 2018

Copy link
Copy Markdown
Contributor

@lihaofd do you think we could temporarily submit this PR and in parallel, debug to find the fix?

@pengzhao-intel

pengzhao-intel commented Aug 8, 2018

Copy link
Copy Markdown
Contributor

@vandanavk, agree, we can submit this PR first and debug in the same time.

@lihaofd please reopen this PR.

@lihaofd

lihaofd commented Aug 9, 2018

Copy link
Copy Markdown
Author

The PR can't be reopened as my local branch has been recreated. Submit new one on #12099. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lihaofd @vandanavk @pengzhao-intel